[FIX] html_editor: link popover not hidden on clicking other element #4762
Conversation
|
This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg-next, it needs to be retargeted before it can be merged. |
ged-odoo
force-pushed
the
master-mysterious-egg-next
branch
from
6e36b56 to
48edd7d
Compare
dabo-odoo
force-pushed
the
master-mysterious-egg-dabo-link-popover-not-disappearing-on-click-other-element
branch
3 times, most recently
from
9930a96 to
e6979cf
Compare
| // Some elements can be clicked on without the selection being changed | ||
| // In those cases the overlay closing and opening must be done manually | ||
| this.addDomListener(this.editable, "click", (ev) => { |
There was a problem hiding this comment.
Hello, I think this is not the right place to fix the issue. The root cause is that when clicking some elements, the selection is not changed. Most likely, they prevented default on these elements. This influences all the selection change handlers.
For example, with the current fix, when you select text from Home button and the tool bar will popup, then you click the Administrator drop down, the toolbar stays open.
IMO, we should check why the selection change is prevented and make the fix there 🤔
Also we already have a click event listener, so if you still need to do the changes at click, you can probably move the code there :)
There was a problem hiding this comment.
Ok, I thought the selection not changing when clicking on certain elements was intended behaviour, I will fix that and it should also fix the popup not closing
There was a problem hiding this comment.
@Jinjiu96 I found out that the selectionchanged event does not fire if the element has contenteditable="false", since if a not editable element is clicked the selection does not move.
I tried solving this by manually moving the cursor (to trigger selectionchanged) if the clicked on element is not editable, let me know if it's acceptable.
I decided to move the cursor to the closest container, but that is completely arbitrary and if you have any better idea let me know.
There was a problem hiding this comment.
Yeah... moving the cursor sounds kinda hacky, maybe the overlay in the editable should be closed when clicking on the contenteditable="false" elements? Then the toolbar and link popover will be both properly closed?
Also you need to retarget to master IIRC
ged-odoo
force-pushed
the
master-mysterious-egg-next
branch
from
2e9b9d5 to
8c5e439
Compare
Because when we change the link element in the popover and apply, we always close the current one and open a new one, the loading at the apply is redundant. We do the reopening to let the overlay plugin reposition popover, and let browser handle the relative/root-relative url to add the domain.
ged-odoo
force-pushed
the
master-mysterious-egg-next
branch
from
8c5e439 to
2e0c6b5
Compare
dabo-odoo
force-pushed
the
master-mysterious-egg-dabo-link-popover-not-disappearing-on-click-other-element
branch
from
e6979cf to
07dad12
Compare
This commit contains multiple changes squashed together, coming from various pull requests on master-mysterious-egg. These changes includes: - No image in colorpicker and working gradient editor for preset background - Clean carousel related tests, remove repeated ones and merge/rename files - Fix logo adding/removing option - Use only "color" tabs in countdown color picker options - Add support for compact hex colors (e.g. #fff) - Fix color styling - Fix setExtraStep - Auto-optimize image upon replace media - Adapt extra product image to also rely on openMediaDialog - Use correct mimetype fields from dataset - Adapt tests - Refresh Dynamic Snippet Carousel when scrolling mode is changed Co-authored-by: Alessandro Lupo <[email protected]> Co-authored-by: Benoit Socias <[email protected]> Co-authored-by: divy-odoo <[email protected]> Co-authored-by: emge-odoo <[email protected]> Co-authored-by: FrancoisGe <[email protected]> Co-authored-by: Jinjiu Liu <[email protected]> Co-authored-by: Keval Bhatt <[email protected]> Co-authored-by: Serhii Rubanskyi - seru <[email protected]>
ged-odoo
force-pushed
the
master-mysterious-egg-next
branch
from
2e0c6b5 to
8478118
Compare
Before this commit clicking on certain elements with a link popover open would not cause the link popover to close. Steps to reproduce: - go to the website homepage - open the editor - click on any top menu links like Home or Contact Us - click on the user dropdwon in the topbar (usually Administrator) - the popover from the first click is still open After the change clicking anywere that's not the popover's current link will cause the popover to close.
dabo-odoo
force-pushed
the
master-mysterious-egg-dabo-link-popover-not-disappearing-on-click-other-element
branch
from
07dad12 to
ce2b531
Compare
ged-odoo
force-pushed
the
master-mysterious-egg-next
branch
4 times, most recently
from
7b6e21e to
921bf70
Compare
|
retargeted to master at odoo#212041 |
Before this commit clicking on certain elements with a link popover
open would not cause the link popover to close.
Steps to reproduce:
After the change clicking anywere that's not the popover's current
link will cause the popover to close.